Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added an experimental convert scripts option for godot3 export #11863

Merged
merged 1 commit into from
Oct 11, 2017

Conversation

marcelofg55
Copy link
Contributor

@marcelofg55 marcelofg55 commented Oct 5, 2017

If enabled this option will convert gdscripts using regexp. So far it converts:

  • _fixed_process( => _physics_process(
  • _pos( => _position(
  • KEY_RETURN => KEY_ENTER
  • RawArray() => PoolByteArray()
  • Vector2Array() => PoolVector2Array()
  • .get_opacity() => .get_modulate().a
  • .set_opacity(var) => .modulate.a = var
  • .connect( => .connect_to_host( // (A HTTPClient object is detected first)
  • ReferenceFrame => Control
  • var.type == InputEvent.KEY => var.get_name() == "InputEventKey"
  • var.type == InputEvent.MOUSE_MOTION => var.get_name() == "InputEventMouseMotion"
  • var.type == InputEvent.MOUSE_BUTTON => var.get_name() == "InputEventMouseButton"
  • var.type == InputEvent.JOYSTICK_MOTION => var.get_name() == "InputEventJoypadMotion"
  • var.type == InputEvent.JOYSTICK_BUTTON => var.get_name() == "InputEventJoypadButton"
  • is_move_and_slide_on_floor() => is_on_floor()
  • is_move_and_slide_on_ceiling() => is_on_ceiling()
  • is_move_and_slide_on_wall() => is_on_wall()

@ghost ghost added this to the 2.1 milestone Oct 6, 2017
@marcelofg55 marcelofg55 force-pushed the conv_scripts_exp branch 2 times, most recently from aec1484 to 7e15835 Compare October 7, 2017 16:02
@Zylann
Copy link
Contributor

Zylann commented Oct 8, 2017

set_opacity() conversion must use current color, not white. (If you assume that any set_opacity function is called on an object having a modulate)

@reduz
Copy link
Member

reduz commented Oct 8, 2017 via email

@Zylann
Copy link
Contributor

Zylann commented Oct 8, 2017

You could also rename "len" identifiers into "length", because "len" is now a reserved function keyword (if there is no other variable in the same scope named "length" already)

@Zylann
Copy link
Contributor

Zylann commented Oct 8, 2017

@reduz what's going on? :D

@Zireael07
Copy link
Contributor

Another candidate is FixedMaterial -> whatever the 3.0 equivalent is.

@Zylann
Copy link
Contributor

Zylann commented Oct 8, 2017

Also AABB -> Rect3, Matrix -> Basis, ColorFrame -> ColorRect, Mesh -> ArrayMesh

@Zylann
Copy link
Contributor

Zylann commented Oct 8, 2017

Maybe also .type == InputEvent.*** => is ***

@HummusSamurai
Copy link
Contributor

HummusSamurai commented Oct 9, 2017

When this is discussed for merge on IRC, I'd like to suggest that it be merged directly without waiting for all the changes to be added.

That way further additions to the conversion script can be contributed through subsequent PRs and help decentralise the effort.

@akien-mga
Copy link
Member

akien-mga commented Oct 9, 2017

For opacity, you can do changes in a non-destructive way (currently you're overriding potential modulate colors) with properties:

self.modulate.a = var

It would still overwrite the alpha channel of a potential modulate color, but we can assume that users were not using both opacity and modulate alpha at the same time.
Or we could do self.modulate.a *= var if we really want to be on the safe side :)

@marcelofg55
Copy link
Contributor Author

@Zylann

You could also rename "len" identifiers into "length", because "len" is now a reserved function keyword (if there is no other variable in the same scope named "length" already)

I thought about that one too, but it probably is a tricky one since you have to avoid getting false positives as len is a pretty short and common word. But I'll give it a try later.

Also AABB -> Rect3, Matrix -> Basis, ColorFrame -> ColorRect, Mesh -> ArrayMesh

Are these direct conversions? I haven't used them myself in gdscript, so idk.

Maybe also .type == InputEvent.*** => is ***

Added :).

@marcelofg55
Copy link
Contributor Author

@akien-mga Just fixed that modulate thing, thanks for the tip :).

@marcelofg55 marcelofg55 changed the title [WIP] Added an experimental convert scripts option for godot3 export Added an experimental convert scripts option for godot3 export Oct 9, 2017
@reduz
Copy link
Member

reduz commented Oct 9, 2017

this looks great, i think, if not implemented already, animations should also be properly fixed

}
regexp.clear();

// Convert var.type == InputEvent.KEY => var.get_name() == "InputEventKey"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the recommended method for type checking.
Use var is InputEventKey instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fixed it.

@akien-mga akien-mga merged commit b1ea083 into godotengine:2.1 Oct 11, 2017
@marcelofg55 marcelofg55 deleted the conv_scripts_exp branch October 12, 2017 02:01
regexp.clear();

// Convert .set_opacity(var) => .modulate.a = var
regexp.compile("([ \t]*)([a-zA-Z0-9_]*)[ ]*\\.set_opacity\\((.*)\\)(.*)");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcelofg55 I'm doing some improvements to the script conversion, and wonder about this regexp. What's the point of [ ]* before \\.set_opacity\\(? Shouldn't it simply be "(.*)\\.set_opacity\\((.*)\\)(.*)"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember if there was a reason that I wrote that regexp that way, but it should work fine as you say as (.*)\\.set_opacity\\((.*)\\)(.*).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants